Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set analytics custom dimensions from govuk: meta tags #558

Merged
merged 3 commits into from Mar 13, 2015
Merged

Conversation

fofr
Copy link
Contributor

@fofr fofr commented Mar 13, 2015

Related to alphagov/slimmer#119

  • Find govuk: namespaced meta tags and use the content of these tags to set dimensions
  • Be explicit about what's being read and set
  • Works with both old and new slimmer versions but can be simplified once apps are all on version 8.0.0 or greater

* Find `govuk:` namespaced meta tags
* Use content of these tags to set dimensions
* Be explicit about what's being read and set
* Works with both old and new slimmer versions
Only version below 8.0.0 insert these old custom variables. When all
apps have been updated to use a modern slimmer this template can be
removed entirely.
@@ -78,6 +79,26 @@
tracker.setDimension(customVar[1], customVar[3], customVar[2], customVar[4]);
}

function setDimensionsFromMetaTags() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason this method can't be added to the prototype? That would save the awkward apply(this) line above and would better follow the approach we suggest in the JavaScript styleguide.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw no reason to add setDimensionsFromMetaTags to the public api as its only use is during initialisation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding it to the public api would let us add proper unit tests to just this method. Which if it is private we can't do. Also while there is no need for this to be added to the public api there is also no harm in it being public. I would lean on the side of making JavaScript methods public unless there is a good reason not to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in 11f0014

* Avoid need for `.apply`
* Be more lenient in what can access this method
edds added a commit that referenced this pull request Mar 13, 2015
Set analytics custom dimensions from `govuk:` meta tags
@edds edds merged commit 2dbfc63 into master Mar 13, 2015
@edds edds deleted the slimmer-meta-tags branch March 13, 2015 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants